-
Notifications
You must be signed in to change notification settings - Fork 0
Add COLLECT_OTEL_*_PORT #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
curusarn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're openning these ports via helm chart config - should the same value be automatically used and picked up by vector to listen on the same ports?
How many steps do I need to listen on port of my choice?
- expose them here via helm config
- then configure the same ports in UI?
I'd love to make this a single step.
E.g. set an ENV and automatically listen on the port if the env is set (?)
|
@curusarn fair points. We haven't added configuring ports to the UI. Do you want to do that (and pass them in through ENV to Vector)? |
|
We could do like OTEL_GRPC_PORT and OTEL_HTTP_PORT in all of install.sh, chart (exposed as otel_grpc_port and otel_http_port), and Vector, with Vector falling back to defaults |
I feel strongly that they should be configured from a single place. Coordinating two places seems extra. I thought is needs to be from helm chart values so that they are known at deploy time. I'm assuming we can't really set port in UI because the ports need to be set on the DaemonSet during deploy time. Or is there a way we can control this from UI fully? |
|
I can't think of one, they need to be known at deploy time. Your plan sounds good, I'll do that 👍 |
|
Thank you! 🙏 |
|
@curusarn this should be much simpler - just take the values and forward into the container. The rest of the support goes into Telemetry. |
curusarn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
For OTel listeners, but I'm thinking forwards - make it flexible enough to be re-used in the future. So not
expose_otelbool.